test(continuation): remove flaky privacy byte-scan test#789
Merged
Conversation
Remove `test_bundle_carries_no_touched_cell_values`. It scanned the serialized bundle for the 8-byte window `[marker, 0,0,0,0,0,0,0]` to assert no touched-cell value leaks, but the check is unsound and flaky: - The bundle is non-deterministic. Grinding derives the nonce via `into_par_iter().find_any()` (crypto/stark/src/grinding.rs), returning a different valid nonce per run, which reseeds Fiat-Shamir and changes every FRI query position and thus the opened bytes serialized. - The needle is produced abundantly by legitimate content, not just a leak. A leaked private cell value is a single byte (memory is `PagedMem<u8>`), so its only signature is `[b, 0x00 x7]` -- 1 distinct byte plus 7 forced zeros. Those trailing zeros are exactly what every small bincode `usize` length prefix (e.g. a table's column count) and small field value already has, so the window collides with honest proof bytes. Empirically, byte value 194 (a column count) appears as this window 18x per proof, every run; whether an arbitrary marker byte collides is pure luck of the trace/serialization layout. The "astronomically unlikely" premise is therefore wrong: it treats the marker as a rare 8-byte value when it is really a 1-byte value against 7 ubiquitous zeros. No marker choice fixes this given byte-granular cells, and a larger multi-byte marker cannot help either -- the old `boundary` serialization emitted each cell interleaved with its epoch/timestamp fields, so a multi-byte value would leak as scattered single-byte windows, not a contiguous searchable run. The real guarantee is already deterministic and enforced at compile time: `InitClaim`/`FiniClaim`/`CellBoundary` have their serde derives deliberately stripped (prover/src/tables/local_to_global.rs), so re-introducing the leak is a compile error rather than something a runtime byte-scan must catch. The removed test added flake without adding coverage the compiler does not already provide.
diegokingston
approved these changes
Jul 6, 2026
jotabulacios
approved these changes
Jul 6, 2026
diegokingston
added a commit
that referenced
this pull request
Jul 6, 2026
…mination) Conflict: crypto/stark/src/fri/mod.rs — union the two new module decls (this PR's `pub mod mmcs;` + #729's `pub(crate) mod terminal;`). Semantic: #729 replaced StarkProof.fri_last_value with fri_final_poly_coeffs (early-termination). Auto-merge reconciled it everywhere except the batched verifier's synthetic placeholder proof (batched_synthetic_table_proof), where fri_last_value: zero() -> fri_final_poly_coeffs: Vec::new(). The two FRI representations now coexist cleanly: non-batched StarkProof uses #729's early-termination coeffs; BatchedMultiProof keeps its own unified fri_last_value. Because main's non-batched multi_prove was ported verbatim, #729's changes to it applied cleanly through this merge. Validated: stark lib 211/211, continuation 25/25 (25: #789 removed the flaky privacy byte-scan test), clean build on default/disk-spill/cuda, fmt clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes
test_bundle_carries_no_touched_cell_values(added in #758). The test is flaky and its detection method is unsound — it adds intermittent CI failures without adding coverage the compiler doesn't already provide.What the test did
It placed distinctive marker bytes (
0xC7/0xC8/0xC9) in the private input, proved a continuation bundle, and asserted none of them appear as the 8-byte window[marker, 0,0,0,0,0,0,0]in the serialized bundle — meant to catch a regression of the touched-cell value leak that #758 fixed (pre-fix,EpochProof.boundaryserialized each cell'sinit.value/fini.value, and a private first-read'sinit.valueis a private byte).Why it's flaky and unsound
1. The bundle is non-deterministic. Grinding derives the PoW nonce via
into_par_iter().find_any()(crypto/stark/src/grinding.rs), which returns a different valid nonce each run. That nonce is folded into the transcript, so every FRI query position (and thus the trace bytes opened into the proof) changes run-to-run. Measured: two proofs of the identical input differ in ~133K of ~2.78M bytes.2. The needle is produced abundantly by legitimate content, not just a leak. Memory is byte-granular (
build_initial_image_paged -> PagedMem<u8>), so a leaked cell value is always a single byteb ∈ [0,255]; its only signature is[b, 0x00 ×7]— one distinctive byte plus seven forced zeros. Those trailing zeros are exactly what every small bincodeusizelength prefix (aVec's length, i.e. a table's column count) and every small field value already carry. So the window collides with honest proof bytes:[194, 0×7]18× per proof, deterministically, every run. Had a marker been0xC2, the test would fail 100% of the time.0xC7/0xC8/0xC9hit 0/90 trials — while the reporter sees0xC8fail ~60% in their environment. Same test, same intent: 0% here, ~60% there, 100% for the adjacent value — the signature of a fragile test.The "astronomically unlikely" premise in the original comment is the flaw: it treats the marker as a rare 8-byte value, when it's really a 1-byte value against 7 ubiquitous zeros.
3. No marker choice fixes it. A larger multi-byte marker can't help either: the old
boundaryserialization emitted each cell interleaved with itsepoch/timestampfields, so a multi-byte value would leak as scattered single-byte windows, never a contiguous searchable run. Given byte-granular cells, the forced-zeros problem is intrinsic.Why removing it is safe
The real guarantee is already deterministic and enforced at compile time:
InitClaim/FiniClaim/CellBoundaryhave theirserdederives deliberately stripped (prover/src/tables/local_to_global.rs), so re-introducing the leak is a compile error — not something a runtime byte-scan must catch. The other continuation privacy/regression tests (multi-page private input, reorderedtouched_page_bases, oversizednum_private_input_pages) are unaffected.Testing
cargo test -p lambda-vm-prover --lib --no-run— compiles clean.cargo clippy -p lambda-vm-prover --lib --tests— clean.cargo fmt— no changes.